-
Notifications
You must be signed in to change notification settings - Fork 3
Upgrade to boost 1.89.0 #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "SHA1 to hex string" bit could be simpler and less error prone.
Why not reinterpret cast hash to const unsigned char* given it's POD and walk through the LUT byte by byte instead of having the two disinct codepaths.
Also (but I think we can ignore this for the time being) SHA-1 digest bytes are big-endian - boost's digest_type uses the underlying host endianess so this would produce broken hashes there (also the previous code was broken in that regard tbh)
Boost 1.89.0 is required to have a compatible standard library. Older versionis would use std::unary_function which is not supported anymore in C++17. Using the proper internal type name boost::uuids::detail::sha1::digest_type result in 2 different behaviors depending on the Boost version. The type can either be an array of "unsigned int[5]" (older Boost) or "char[20]" (newer Boost) which the serialization code needs to handle. The serialization code is also moved away from using sprintf as modern compilers will emit lots of different warnings or errors.
|
The less error prone thing to do would be not to use the internals of Since the code path with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| hash_buf[i * 2 + 7] = int_to_hex[(hash[i] ) & 0xF]; | ||
| } | ||
| } else { | ||
| throw std::runtime_error("Unsupported boost::uuids::detail::sha1::digest_type size"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should set ec here not throw, throw is only from the overload without ec, this is following the same pricinple as how boost.asio design APIs
If we could do static_assert it would be better, but it seems not possible in our c++ version.
Boost 1.89.0 is required to have a compatible standard library. Older versionis would use std::unary_function which is not supported anymore in C++17.
Using the proper internal type name boost::uuids::detail::sha1::digest_type result in 2 different behaviors depending on the Boost version. The type can either be an array of "unsigned int[5]" (older Boost) or "char[20]" (newer Boost) which the serialization code needs to handle.
The serialization code is also moved away from using sprintf as modern compilers will emit lots of different warnings or errors.